@geordie666, one question:

with the on-going discussion in the twin desitarget PR, I realize I may have mis-interpreted one thing.
let s take dark, dark1b for example here.

is the following the approach we want:

• for a dark tile: only query the dark ledgers
• for a dark1b tile: query both the dark and dark1b ledgers.
  right?

in my current coding, I query both (dark, dark1b) ledgers for a dark tile, so that s likely a mistake -- I ll correct that tomorrow when nersc is back up.

Yes, that's correct. It's a little hack-y but, now:

DARK means only DARK.
DARK1B means DARK1B and DARK (suitably merged).

perfect, thanks for the confirmation -- and sorry I forgot about that, you surely already explained it to me!

I'm sure I've missed it, but just confirming---

1. when program = dark, we want to continue getting the same results we have always gotten. The dark1b ledgers are ignored. It's okay to have some header new header keywords if it makes the code cleaner and it's clear when they should be ignored, but they don't seem desired.
2. when program = dark1b, we do want to read in both sets of ledgers and merge them, and we do need new header keywords describing the new different new sets of MTLs read in.

My naive reading of this code makes it look like we always read dark/dark1b and bright/bright1b ledgers and merge them, for both the dark and dark1b programs, which wasn't what I thought we wanted.

thanks for the confirmation (and looking at the code!); I confirm the current code is not doing that, ie is wrong for dark tiles.

I ll work on that today, now that nersc is back up, it s an easy fix.

I ll also code the approach discussed here desihub/desitarget#833 (comment) for secondaries.

with the last couple of commits, the code should now correctly handle the querying of the catalogs, as discussed.
that s both for primary and secondary targets.
as I ve "enabled" secondary targets for bright1b,dark1b, but there are none so far, I had to add a check in fba_launch, looking for secondaries only if the ledger folder exists.

@schlafly, I think it s ok but double-checking in case:
for GOALTYPE, I add two new values, DARK1B and BRIGHT1B, that s what we want, right?

for info, I ve propagated the dark1b/bright1b cases in few functions used for the fiberassign-TILEID.png plot, and also in fba_rerun_io.py.

as previously, I ve generated two test tiles here [edit: I ve changed the ra,dec to 180,10 to have full coverage from the LGE test files]:

fba_launch --outdir /global/cfs/cdirs/desi/users/raichoor/fiberassign_dev/fiberassign_desi1b/20250221 --tileid 999990 --tilera 180 --tiledec 10 --survey main --dtver 1.1.1  --program DARK

fba_launch --outdir /global/cfs/cdirs/desi/users/raichoor/fiberassign_dev/fiberassign_desi1b/20250221 --tileid 999991 --tilera 180 --tiledec 10 --survey main --dtver 1.1.1  --program DARK1B

as far as I can say, the fiberassign files look ok to me, ie formatted as we want; of course we d to run more massive tests once the targets are available over the full footprint!

I m just a bit suprised by this https://data.desi.lbl.gov/desi/users/raichoor/fiberassign_dev/fiberassign_desi1b/20250221/999/fiberassign-999991.png plot, where one can see that the assigned LGE do not have the same z-band mag distribution as the parent LGE sample; the assigned ones are brighter (ie the red curve is not ~flat); I m investigating...

mmhh... looks like that the assigned LGE are the ~10% ones which also are BGS_ANY.

I m investigating, but I suspect it s because I didn t propagate the DARK1B in some c++ part of the code.

because those two samples (LGE and LGE & BGS_ANY) have the same PRIORITY, NUMOBS_MORE values, the difference being that LGE mostly have OBSCONDITIONS = ['DARK1B'], and that LGE & BGS_ANY have OBSCONDITIONS=['BRIGHT', 'IGNORE', 'DARK1B'].

ok found the issue: I had to enable the LGE target class in targets.py, I totally forgot about that one!

the results look better now; +1 for qa plots!

I ve moved previous run to https://data.desi.lbl.gov/desi/users/raichoor/fiberassign_dev/fiberassign_desi1b/20250221/v0.
and runs with this last commit are here: https://data.desi.lbl.gov/desi/users/raichoor/fiberassign_dev/fiberassign_desi1b/20250221/v1.

mmhh.. tests are still failing after my small fix in the previous commit.

@geordie666, I m not familiar with those, but could it be that it fails because there are, in the test scripts, lines like:

in which case, there s nothing to do until that desihub/desitarget#833 is merged + a desitarget/tag is created?
then update that line with this new tag:

except that "failing checks" issue, I think I m happy with the current version.
as said, I ll perform further testing when new ledgers/targets will be available over a larger area.

I'm embarrassed, but I don't remember what GOAL TYPE controls. Maybe which variety of effective time is used? Presumably we want to continue using the the normal LRG effective time (i.e., dark). That's less obvious for bright1b, but given that we likely don't want to develop a new effective time, probably we continue to use bright? Or am I missing what GOAL TYPE controls?

I admit, it s not super-clear for me either where GOALTYPE is used; I have same sense as you, ie it s probably used to pick which efftime to choose; I would need to dig into history/codes.

I thought it was used online by the ETC to choose the correct way to estimate the efftime, but I don t see any occurence of GOALTYPE in the code, apart from a check that the keyword is there: https://github.com/search?q=repo%3Adesihub%2Fdesietc%20goaltype&type=code.

for the spectro. pipeline, from a quick check, it s indeed used to pick which EFFTIME_SPEC to use, e.g.:
https://github.com/desihub/desispec/blob/c717572e415314babdf30c09fa2fe4a75af1da4f/bin/desi_tsnr_afterburner#L509-L526

so my 2 cents would be:

• maybe it s better to define GOALTYPE=DARK1B or BRIGHT1B for those two programs, so that we keep the flexibility to define other properties;
• codes like desispec and desietc will probably need to update anyway for the desi-extension, to handle those new programs DARK1B and BRIGHT1B; then the decision e.g. of what EFFTIME to pick for DARK1B and BRIGHT1B will be done in those codes -- and if the decision is to keep the same definitions as for DARK and BRIGHT, no problem, but at least it will be explicitly stated that e.g. a "DARK1B should be treated as DARK" decision is taken...

I hear you, but: conceptually, the dark (extended) LRG and bright BGS targets are still the defining targets of these extension programs. So I don't think we'd consider using a different GOALTYPE.

allright, thanks for the input; I ve removed the 1B from the GOALTYPE.